Skip to content

Add configurable file path navigation for HOCON string values.#90

Open
janresiak wants to merge 1 commit into
AVSystem:masterfrom
janresiak:feature/custom_file_link_rules
Open

Add configurable file path navigation for HOCON string values.#90
janresiak wants to merge 1 commit into
AVSystem:masterfrom
janresiak:feature/custom_file_link_rules

Conversation

@janresiak

Copy link
Copy Markdown

Introduce HoconFilePathReferenceProvider that resolves HOCON string values to project files when they match user-configured extensions.

 Introduce HoconFilePathReferenceProvider that resolves HOCON string values to project files when they match user-configured extensions.
@ddworak ddworak requested a review from halotukozak March 6, 2026 10:16
@ddworak ddworak changed the base branch from idea25 to master March 6, 2026 10:43
@halotukozak halotukozak requested review from Copilot and halotukozak and removed request for halotukozak March 6, 2026 17:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new reference provider to enable navigating from HOCON string values to files in the project, gated by per-project settings (extensions + optional search roots).

Changes:

  • Introduces HoconFilePathReferenceProvider and registers it for HStringValue PSI elements.
  • Adds persistent project settings for file navigation extensions and search roots, including parsing helpers.
  • Extends the project settings UI to let users configure these new options.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
src/org/jetbrains/plugins/hocon/settings/HoconProjectSettingsPanel.java Adds UI fields to configure file-path navigation extensions and search roots.
src/org/jetbrains/plugins/hocon/settings/HoconProjectSettings.scala Persists the new settings and provides list-parsing helpers.
src/org/jetbrains/plugins/hocon/ref/HoconReferenceContributor.scala Registers the new file-path reference provider for HOCON string values.
src/org/jetbrains/plugins/hocon/ref/HoconFilePathReferenceProvider.scala Implements file resolution based on configured extensions and module roots/resource roots.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +53 to +54
val dir = if (searchRoot.isEmpty) contentRoot else contentRoot.findFileByRelativePath(searchRoot)
Option(dir).iterator

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the configured-roots branch, searchRoot.isEmpty is treated specially (falling back to contentRoot), but fileNavigationSearchRootsList currently removes empty roots. This makes the searchRoot.isEmpty branch unreachable and can be removed/simplified (or adjust the settings parsing to preserve empty entries if that behavior is intended).

Suggested change
val dir = if (searchRoot.isEmpty) contentRoot else contentRoot.findFileByRelativePath(searchRoot)
Option(dir).iterator
Option(contentRoot.findFileByRelativePath(searchRoot)).iterator

Copilot uses AI. Check for mistakes.
Comment on lines 14 to 18
def registerReferenceProviders(registrar: PsiReferenceRegistrar): Unit = {
registrar.registerReferenceProvider(pattern[HIncludeTarget], new IncludedFileReferenceProvider)
registrar.registerReferenceProvider(pattern[HStringValue], new HoconPropertiesReferenceProvider)
registrar.registerReferenceProvider(pattern[HStringValue], new HoconFilePathReferenceProvider)
}

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No tests are added for the new file-path reference resolution behavior. There are existing reference-resolution tests in test/org/jetbrains/plugins/hocon/ref, so this would benefit from a test that (1) sets fileNavigationExtensions/fileNavigationSearchRoots, (2) creates a target file under the configured root, and (3) asserts findReferenceAt(...).resolve() returns the expected PsiFile.

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +36
def fileNavigationExtensionsList: List[String] =
if (fileNavigationExtensions.trim.isEmpty) Nil
else fileNavigationExtensions.split(",").map(_.trim).filter(_.nonEmpty).toList

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileNavigationExtensionsList currently returns trimmed tokens as-is, so common inputs like ".sql" (with a leading dot) or mixed-case extensions won't match the resolver logic that checks for endsWith("." + ext). Consider normalizing extensions here (e.g., strip a leading '.', lowercase) so the configured value behaves predictably regardless of whether users type sql or .sql.

Copilot uses AI. Check for mistakes.

def fileNavigationSearchRootsList: List[String] =
if (fileNavigationSearchRoots.trim.isEmpty) Nil
else fileNavigationSearchRoots.split(",").map(_.trim).filter(_.nonEmpty).toList

Copilot AI Mar 6, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileNavigationSearchRootsList filters out empty entries (filter(_.nonEmpty)), but the resolver later has logic for searchRoot.isEmpty to mean "use content root". As written, that branch can never be hit. Either keep empty entries (and define what they mean), or remove the searchRoot.isEmpty handling in the resolver to avoid dead/contradictory logic.

Suggested change
else fileNavigationSearchRoots.split(",").map(_.trim).filter(_.nonEmpty).toList
else fileNavigationSearchRoots.split(",").map(_.trim).toList

Copilot uses AI. Check for mistakes.

@halotukozak halotukozak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check Copilot's comments.
Add tests

private JCheckBox classReferencesQuotedCheckBox;
private JCheckBox propertyReferencesCheckBox;
private JCheckBox searchInGotoSymbol;
private JTextField fileNavigationExtensionsField;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use JBList - you won't have to parse the strings and it will look better


GridBagConstraints gbc = new GridBagConstraints();
gbc.anchor = GridBagConstraints.WEST;
gbc.insets = new Insets(2, 4, 2, 4);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just use JBUI.insets

fileNavPanel.setBorder(BorderFactory.createTitledBorder(
BorderFactory.createEtchedBorder(), "File Path Navigation"));

GridBagConstraints gbc = new GridBagConstraints();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use FormBuilder instead of manual Grid


def fileNavigationExtensionsList: List[String] =
if (fileNavigationExtensions.trim.isEmpty) Nil
else fileNavigationExtensions.split(",").map(_.trim).filter(_.nonEmpty).toList

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplication of logic here and in fileNavigationSearchRootsList


val baseDirs = if (searchRoots.nonEmpty) {
// Use explicitly configured roots (relative paths from content roots)
mrm.getContentRoots.iterator.flatMap { contentRoot =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use for comprehension

val baseDirs = if (searchRoots.nonEmpty) {
// Use explicitly configured roots (relative paths from content roots)
mrm.getContentRoots.iterator.flatMap { contentRoot =>
searchRoots.iterator.flatMap { searchRoot =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.iterator is useless

}
} else {
// Default: use resource root resolved from the project model (Compile / resourceDirectory)
mrm.getContentEntries.iterator.flatMap { entry =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use for comprehension

}
}

baseDirs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use for comprehension

result.nextOption().orNull
}

override def getVariants: Array[AnyRef] = Array.empty

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be val

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants